feat: ignore argument for @updatedAt
#572
Conversation
📝 WalkthroughWalkthroughExtends the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/sdk/src/ts-schema-generator.ts:
- Around line 566-578: The updatedAt attribute handling uses unsafe casts for
ignoreArg.value and items; add explicit type guards to validate that
ignoreArg.value is an ArrayExpr and each item is a ReferenceExpr before mapping:
check with a helper isArrayExpr(ignoreArg.value) and isReferenceExpr(item), and
if validation fails either skip the ignore mapping or throw a clear error
mentioning the field name and attribute (e.g., "invalid @updatedAt(ignore) —
expected array of references"); then only call
ts.factory.createArrayLiteralExpression and use (item as
ReferenceExpr).target.$refText after those guards. Update the block that builds
the 'updatedAt' propertyAssignment to use these guards and produce a clear
error/log instead of letting runtime type assertions throw.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/cli/test/ts-schema-gen.test.tspackages/language/res/stdlib.zmodelpackages/orm/src/client/crud-types.tspackages/orm/src/client/crud/operations/base.tspackages/schema/src/schema.tspackages/sdk/src/ts-schema-generator.tstests/e2e/orm/client-api/update.test.tstests/e2e/orm/schemas/basic/schema.ts
🧰 Additional context used
📓 Path-based instructions (2)
tests/e2e/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
E2E tests should validate real-world schema compatibility with established projects
Files:
tests/e2e/orm/client-api/update.test.tstests/e2e/orm/schemas/basic/schema.ts
**/*.zmodel
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.zmodel: Always runzenstack generateafter modifying ZModel schemas
ZModel schema files should define database structure and policies that compile to TypeScript viazenstack generate
Files:
packages/language/res/stdlib.zmodel
🧠 Learnings (5)
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.test.{ts,tsx} : ORM package tests should include comprehensive client API tests and policy tests
Applied to files:
tests/e2e/orm/client-api/update.test.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/e2e/**/*.{ts,tsx} : E2E tests should validate real-world schema compatibility with established projects
Applied to files:
tests/e2e/orm/client-api/update.test.tspackages/cli/test/ts-schema-gen.test.tstests/e2e/orm/schemas/basic/schema.ts
📚 Learning: 2025-12-30T15:07:06.254Z
Learnt from: mwillbanks
Repo: zenstackhq/zenstack-v3 PR: 550
File: packages/orm/src/client/crud/operations/base.ts:158-159
Timestamp: 2025-12-30T15:07:06.254Z
Learning: Do not use ts-expect-error in production code within the zenstackhq/zenstack-v3 repo (e.g., packages/*). Use explicit type annotations, targeted type assertions, or refactor to resolve the type error. ts-expect-error may be acceptable only in test files for stubbing or temporary silencing. Ensure production code is type-safe and maintainable.
Applied to files:
packages/schema/src/schema.tspackages/orm/src/client/crud/operations/base.tspackages/orm/src/client/crud-types.tspackages/sdk/src/ts-schema-generator.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Implement plugin hooks at ORM, Kysely, and entity mutation levels for query interception and customization
Applied to files:
packages/orm/src/client/crud-types.ts
📚 Learning: 2025-10-21T16:09:31.218Z
Learnt from: ymc9
Repo: zenstackhq/zenstack-v3 PR: 319
File: packages/runtime/src/client/executor/zenstack-query-executor.ts:63-72
Timestamp: 2025-10-21T16:09:31.218Z
Learning: In ZenStack, TypeDefs can be inherited by models. When a TypeDef contains fields with `map` attributes, those mapped field names need to be processed by the QueryNameMapper since they become part of the inheriting model's schema. Therefore, when checking if a schema has mapped names (e.g., in `schemaHasMappedNames`), both `schema.models` and `schema.typeDefs` must be inspected for `@map` and `map` attributes.
Applied to files:
packages/orm/src/client/crud-types.ts
🧬 Code graph analysis (5)
tests/e2e/orm/client-api/update.test.ts (1)
tests/e2e/orm/client-api/utils.ts (1)
createUser(6-21)
packages/cli/test/ts-schema-gen.test.ts (4)
packages/testtools/src/schema.ts (1)
generateTsSchema(40-74)tests/e2e/orm/schemas/basic/schema.ts (1)
schema(292-292)packages/clients/tanstack-query/test/schemas/basic/schema-lite.ts (1)
schema(172-172)samples/next.js/zenstack/schema-lite.ts (1)
schema(106-106)
packages/orm/src/client/crud/operations/base.ts (2)
packages/orm/src/client/query-utils.ts (1)
isScalarField(134-137)packages/common-helpers/src/clone.ts (1)
clone(6-24)
packages/orm/src/client/crud-types.ts (1)
packages/schema/src/schema.ts (2)
GetModelField(156-160)UpdatedAtInfo(62-64)
packages/sdk/src/ts-schema-generator.ts (3)
packages/language/src/utils.ts (1)
getAttribute(59-61)packages/sdk/src/model-utils.ts (1)
getAttribute(66-68)packages/language/src/generated/ast.ts (6)
DataFieldAttribute(348-353)DataFieldAttribute(355-355)ArrayExpr(185-189)ArrayExpr(191-191)ReferenceExpr(688-693)ReferenceExpr(695-695)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-test (22.x, postgresql)
- GitHub Check: build-test (22.x, sqlite)
🔇 Additional comments (9)
packages/schema/src/schema.ts (1)
62-64: LGTM! Clean type definition for the ignore feature.The new
UpdatedAtInfotype and the extension ofFieldDef.updatedAtto acceptboolean | UpdatedAtInfoare well-designed. The use ofreadonly string[]ensures immutability, and the conditional type update inFieldHasDefaultmaintains consistency across the type system.Also applies to: 73-73, 288-288
tests/e2e/orm/schemas/basic/schema.ts (1)
33-38: LGTM! Generated schema correctly reflects the ignore configuration.The generated
updatedAtfield structure with theignorearray matches theUpdatedAtInfotype definition. The inclusion of'id'and'createdAt'in the ignore list provides a practical example for E2E testing.tests/e2e/orm/client-api/update.test.ts (2)
133-167: LGTM! Comprehensive test coverage for ignored fields behavior.This test validates the core feature:
updatedAtshould not change when only ignored fields (id,createdAt) are modified. The test covers multiple scenarios including updating ignored fields individually and in combination, and properly verifies the timestamp remains unchanged by fetching the record after each update.
169-183: LGTM! Validates that non-ignored fields trigger updatedAt.This test ensures the feature doesn't break the default behavior: when any non-ignored field (like
name) is updated alongside ignored fields, theupdatedAttimestamp should still be updated. Good complementary coverage to the previous test.packages/language/res/stdlib.zmodel (1)
406-406: LGTM! Backward-compatible attribute signature update.The addition of the optional
ignore: FieldReference[]?parameter to@updatedAtis well-designed:
- Uses
FieldReference[]for type-safe field references- Optional parameter maintains backward compatibility with existing schemas
- Follows ZModel naming conventions with the underscore prefix
packages/cli/test/ts-schema-gen.test.ts (1)
446-726: LGTM! Comprehensive test coverage for the newignorefeature.The test validates that the generated schema correctly represents
@updatedAt(ignore: [email])with anUpdatedAtInfoobject containing the ignore array. The assertions are thorough and cover both the field structure and the attribute representation.packages/orm/src/client/crud-types.ts (2)
33-33: LGTM! Correct import for the new type.The
UpdatedAtInfoimport is necessary to support the updated type signature at line 984.
984-984: LGTM! Type widening correctly handles the newUpdatedAtInfostructure.The change from
extends truetoextends (true | UpdatedAtInfo)properly accounts for the new schema whereupdatedAtcan be either a boolean or an object with ignore configuration. This ensures that fields with@updatedAtremain optional during create operations regardless of the ignore configuration.packages/orm/src/client/crud/operations/base.ts (1)
997-1005: LGTM! Correct implementation of conditional timestamp updates.The logic correctly implements the
ignorefeature:
- Identifies which scalar fields are being updated
- Constructs an ignore set from the configuration
- Only updates the timestamp if at least one non-ignored scalar field is being updated
Edge cases are handled correctly:
- Only ignored fields updated → timestamp NOT updated ✓
- Only relation fields updated → timestamp NOT updated ✓
- Empty updates → timestamp NOT updated ✓
- Mix of ignored/non-ignored → timestamp updated ✓
|
@coderabbitai why is the test failing in CI when it passes locally? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 1116 🏁 Script executed: Length of output: 285018 🏁 Script executed: Length of output: 32454 @sanny-io Thanks for the logs — the failures are coming from the CI Postgres setup, not from the updatedAt ignore logic. What’s different in CI vs local
Why it passes locally
How to fix Option A — Align connection strings to user=postgres
Option B — Create the root role in the CI Postgres container
Quick checks you can make
If you want, I can prep a small PR to:
Let me know which option you prefer and I’ll provide the exact patch. |
|
What an absolute adventure lol. This should be ready now... @ymc9 how are the e2e test schemas supposed to be handled? Am I supposed to run the |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/sdk/src/ts-schema-generator.ts:
- Around line 574-582: The argument lookup for the updatedAt attribute uses
arg.name but the project consistently stores argument names on
arg.$resolvedParam.name; update the find call in the updatedAt handling so it
checks arg.$resolvedParam.name === 'ignore' (locate the block using
updatedAtAttrib, getAttribute, DataFieldAttribute and the
createPropertyAssignment that pushes into objectFields) and leave the rest
(createUpdatedAtObject/ts.factory.createTrue) unchanged so the ignore argument
is correctly detected.
🧹 Nitpick comments (1)
packages/sdk/src/ts-schema-generator.ts (1)
524-530: Consider adding defensive checks for the type assertions.The type assertions
as ArrayExprandas ReferenceExprassume the schema validation guarantees the correct structure. While this is likely true, adding runtime guards would make the code more robust against future schema changes or unexpected inputs.♻️ Optional defensive implementation
private createUpdatedAtObject(ignoreArg: AttributeArg) { + const arrayExpr = ignoreArg.value; + if (!isArrayExpr(arrayExpr)) { + throw new Error('Expected array expression for @updatedAt ignore argument'); + } return ts.factory.createObjectLiteralExpression([ ts.factory.createPropertyAssignment('ignore', ts.factory.createArrayLiteralExpression( - (ignoreArg.value as ArrayExpr).items.map((item) => ts.factory.createStringLiteral((item as ReferenceExpr).target.$refText)) + arrayExpr.items.map((item) => { + if (!isReferenceExpr(item)) { + throw new Error('Expected reference expression in @updatedAt ignore list'); + } + return ts.factory.createStringLiteral(item.target.$refText); + }) )) ]); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/schema/src/schema.tspackages/sdk/src/ts-schema-generator.ts
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-12-30T15:07:06.254Z
Learnt from: mwillbanks
Repo: zenstackhq/zenstack-v3 PR: 550
File: packages/orm/src/client/crud/operations/base.ts:158-159
Timestamp: 2025-12-30T15:07:06.254Z
Learning: Do not use ts-expect-error in production code within the zenstackhq/zenstack-v3 repo (e.g., packages/*). Use explicit type annotations, targeted type assertions, or refactor to resolve the type error. ts-expect-error may be acceptable only in test files for stubbing or temporary silencing. Ensure production code is type-safe and maintainable.
Applied to files:
packages/schema/src/schema.tspackages/sdk/src/ts-schema-generator.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to **/*.zmodel : ZModel schema files should define database structure and policies that compile to TypeScript via `zenstack generate`
Applied to files:
packages/sdk/src/ts-schema-generator.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/e2e/**/*.{ts,tsx} : E2E tests should validate real-world schema compatibility with established projects
Applied to files:
packages/sdk/src/ts-schema-generator.ts
📚 Learning: 2025-10-21T16:09:31.218Z
Learnt from: ymc9
Repo: zenstackhq/zenstack-v3 PR: 319
File: packages/runtime/src/client/executor/zenstack-query-executor.ts:63-72
Timestamp: 2025-10-21T16:09:31.218Z
Learning: In ZenStack, TypeDefs can be inherited by models. When a TypeDef contains fields with `map` attributes, those mapped field names need to be processed by the QueryNameMapper since they become part of the inheriting model's schema. Therefore, when checking if a schema has mapped names (e.g., in `schemaHasMappedNames`), both `schema.models` and `schema.typeDefs` must be inspected for `@map` and `map` attributes.
Applied to files:
packages/sdk/src/ts-schema-generator.ts
🧬 Code graph analysis (1)
packages/sdk/src/ts-schema-generator.ts (5)
packages/schema/src/schema.ts (1)
AttributeArg(45-48)packages/language/src/generated/ast.ts (8)
AttributeArg(214-219)AttributeArg(221-221)ArrayExpr(187-191)ArrayExpr(193-193)ReferenceExpr(690-695)ReferenceExpr(697-697)DataFieldAttribute(350-355)DataFieldAttribute(357-357)packages/sdk/src/prisma/prisma-builder.ts (1)
AttributeArg(248-257)packages/language/src/utils.ts (1)
getAttribute(59-61)packages/sdk/src/model-utils.ts (1)
getAttribute(66-68)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-test (22.x, postgresql)
- GitHub Check: build-test (22.x, sqlite)
🔇 Additional comments (3)
packages/schema/src/schema.ts (3)
62-64: LGTM!The
UpdatedAtInfotype is well-defined with appropriate use ofreadonlyfor immutability. This cleanly models the optionalignoreconfiguration for@updatedAt.
73-73: LGTM!The union type
boolean | UpdatedAtInfois backwards compatible and allows for the new ignore-aware configuration.
283-293: LGTM!The updated conditional correctly identifies fields with
@updatedAtas having a default, regardless of whetherignoreis specified. BothtrueandUpdatedAtInfowill satisfy theextendscheck.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @packages/sdk/src/ts-schema-generator.ts:
- Around line 524-530: The createUpdatedAtObject method performs unsafe casts on
ignoreArg.value and on each item; add proper type guards (use
isArrayExpr(ignoreArg.value) and isReferenceExpr(item)) before mapping to
strings, handle non-array or non-reference cases (e.g., produce an empty array
or skip invalid items) and keep the output as a
ts.factory.createArrayLiteralExpression of string literals; update
createUpdatedAtObject to validate the value and items and fall back safely
instead of using direct casts.
- Around line 574-582: The predicate that searches attribute args reads
arg.$resolvedParam.name without guarding for undefined (in the updatedAtAttrib
-> ignoreArg find), which can throw; change the predicate to use optional
chaining (arg.$resolvedParam?.name === 'ignore') so ignoreArg is safely computed
and keep the existing usage with createUpdatedAtObject / ts.factory.createTrue
unchanged; apply the same optional-chaining fix to all other find predicates in
this file that access $resolvedParam.name (the other unsafe occurrences flagged)
to ensure null-safety across getAttribute handling.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/sdk/src/ts-schema-generator.ts
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-12-30T15:07:06.254Z
Learnt from: mwillbanks
Repo: zenstackhq/zenstack-v3 PR: 550
File: packages/orm/src/client/crud/operations/base.ts:158-159
Timestamp: 2025-12-30T15:07:06.254Z
Learning: Do not use ts-expect-error in production code within the zenstackhq/zenstack-v3 repo (e.g., packages/*). Use explicit type annotations, targeted type assertions, or refactor to resolve the type error. ts-expect-error may be acceptable only in test files for stubbing or temporary silencing. Ensure production code is type-safe and maintainable.
Applied to files:
packages/sdk/src/ts-schema-generator.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to **/*.zmodel : ZModel schema files should define database structure and policies that compile to TypeScript via `zenstack generate`
Applied to files:
packages/sdk/src/ts-schema-generator.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/e2e/**/*.{ts,tsx} : E2E tests should validate real-world schema compatibility with established projects
Applied to files:
packages/sdk/src/ts-schema-generator.ts
📚 Learning: 2025-10-21T16:09:31.218Z
Learnt from: ymc9
Repo: zenstackhq/zenstack-v3 PR: 319
File: packages/runtime/src/client/executor/zenstack-query-executor.ts:63-72
Timestamp: 2025-10-21T16:09:31.218Z
Learning: In ZenStack, TypeDefs can be inherited by models. When a TypeDef contains fields with `map` attributes, those mapped field names need to be processed by the QueryNameMapper since they become part of the inheriting model's schema. Therefore, when checking if a schema has mapped names (e.g., in `schemaHasMappedNames`), both `schema.models` and `schema.typeDefs` must be inspected for `@map` and `map` attributes.
Applied to files:
packages/sdk/src/ts-schema-generator.ts
🧬 Code graph analysis (1)
packages/sdk/src/ts-schema-generator.ts (5)
packages/schema/src/schema.ts (1)
AttributeArg(45-48)packages/language/src/generated/ast.ts (8)
AttributeArg(214-219)AttributeArg(221-221)ArrayExpr(187-191)ArrayExpr(193-193)ReferenceExpr(690-695)ReferenceExpr(697-697)DataFieldAttribute(350-355)DataFieldAttribute(357-357)packages/sdk/src/prisma/prisma-builder.ts (1)
AttributeArg(248-257)packages/language/src/utils.ts (1)
getAttribute(59-61)packages/sdk/src/model-utils.ts (1)
getAttribute(66-68)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-test (22.x, sqlite)
- GitHub Check: build-test (22.x, postgresql)
1 participant
Allows the
@updatedAtattribute to accept an array of field references. An update which only contains ignored fields does not update the timestamp.Prisma issue: prisma/prisma#24080
Sample Usage
Summary by CodeRabbit
New Features
Behavior
Tests
✏️ Tip: You can customize this high-level summary in your review settings.